Skip to content

feat(fog): volumetric fog#2361

Merged
davo0411 merged 27 commits into
community-shaders:devfrom
jiayev:volumetric-fog
Jun 3, 2026
Merged

feat(fog): volumetric fog#2361
davo0411 merged 27 commits into
community-shaders:devfrom
jiayev:volumetric-fog

Conversation

@jiayev

@jiayev jiayev commented May 17, 2026

Copy link
Copy Markdown
Collaborator

This pull request introduces a comprehensive implementation of volumetric fog for the Exponential Height Fog system. The changes add support for 3D volumetric fog grids, new compute shaders for fog material, conservative depth, and light integration, and a variety of utility functions and settings to control the volumetric fog rendering pipeline. The update also refactors and expands the fog shader code to support both analytical and volumetric fog blending, with support for VR and various quality/performance settings.

The most important changes are:

Volumetric Fog Feature Implementation

  • Added volumetric fog compute shaders: Introduced new compute shaders for material evaluation (VolumetricFogMaterialCS.hlsl), conservative depth calculation (VolumetricFogConservativeDepthCS.hlsl), and light scattering integration (VolumetricFogIntegrationCS.hlsl). These shaders implement the core volumetric fog rendering pipeline. [1] [2] [3]
  • Added shared volumetric fog utility code: Created VolumetricFogCommon.hlsli and VolumetricFogCSCommon.hlsli with reusable functions for grid coordinate conversions, fog parameter calculations, and the Henyey-Greenstein phase function, centralizing volumetric fog math and logic. [1] [2]

Shader and API Extensions

  • Refactored main fog shader to support volumetric fog: Updated ExponentialHeightFog.hlsli to sample and blend volumetric fog with analytical fog, handle VR, and expose new utility functions for sampling and combining fog. The Henyey-Greenstein function was moved to the common file. [1] [2] [3]
  • Extended shared data structure: Added new volumetric fog settings and parameters to the exponentialHeightFogSettings struct in SharedData.hlsli, enabling control over grid size, distances, albedo, extinction, and more.

Configuration

  • Bumped Exponential Height Fog feature version: Updated the feature version in the .ini to 1-3-0 to reflect the new volumetric fog capabilities.

Summary by CodeRabbit

  • New Features

    • Volumetric exponential height fog: froxel-based light scattering, shadows, conservative depth, integration, and temporal reprojection for improved stability and visuals (including stereo/VR-aware sampling).
    • API: explicit no-volumetric/reflection-safe fog paths for consistent results in reflections and water.
  • User Interface

    • Runtime controls for volumetric grid, distances/fade, extinction/albedo/emissive, scattering, history/jitter, upsample jitter, and local-light intensity plus debug options.
  • Documentation

    • Version bumped to 1-3-0.
  • Chores

    • Directional inscattering default adjusted.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a froxel-based volumetric exponential-height-fog pipeline: new HLSL helpers and compute shaders for material, conservative depth, light scattering, and integration; refactors exponential fog shader to blend volumetric and analytical results; updates pixel shaders to use screen-position-aware fog calls; and extends C++ runtime for resource management, dispatch, history, and UI.

Changes

Volumetric Fog Pipeline

Layer / File(s) Summary
Volumetric Fog HLSL Foundation
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogCommon.hlsli
Mathematical helpers for volumetric fog: density/falloff accessors, grid Z sizing (with compile-time override), depth-distribution scaling, slice depth/coordinate conversions, height-based extinction evaluation, and Henyey–Greenstein phase function.
Volumetric Grid Constants & Coordinate Helpers
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogCSCommon.hlsli
Constant buffer at b0 with grid-sizing macros, feature flags, inverse-grid metrics, history/jitter parameters; utility functions for grid bounds checking and stereo-aware world-space cell positioning.
Volumetric Material & Conservative Depth Setup
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogMaterialCS.hlsl, features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogConservativeDepthCS.hlsl
Two compute shaders: material evaluates height-based extinction/scattering per grid cell; conservative depth computes maximum screen-space depth per 2D grid position for occlusion testing.
Volumetric Light Scattering Computation
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl
Per-froxel light scattering shader: directional shadow sampling with cascade PCF and secondary-cascade blending, optional world shadowing (terrain/clouds), sky scattering with IBL/skylighting occlusion, conditionally-compiled local light accumulation via clustered lights, temporal reprojection with history validation/fixup, and jittered temporal blending with NaN/Inf sanitization.
Volumetric Light Integration
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogIntegrationCS.hlsl
Vertical integration of light scattering along grid Z-axis: applies near-distance fade-in, accumulates lighting and transmittance per layer, outputs integrated fog per 2D grid position.
Analytical Fog Refactor & Volumetric Blending
features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli
Refactors exponential height fog into internal function supporting both world-position and screen-position paths; adds VR eye selection, volumetric sampling helpers, transmittance-based blending, and screen-position-aware GetExponentialHeightFog overload.
Shader Pass Updates for Volumetric Blending
package/Shaders/Effect.hlsl, package/Shaders/ISSAOComposite.hlsl, package/Shaders/Lighting.hlsl, package/Shaders/Water.hlsl, package/Shaders/DistantTree.hlsl, package/Shaders/Sky.hlsl
Updates pixel shaders to use new screen-position-aware GetExponentialHeightFog variant, constructing screen-position from stereo UVs and dynamic-resolution scaling; adjusts blend ordering to use returned transmittance.
C++ Feature Implementation & Runtime Support
src/Features/ExponentialHeightFog.h, src/Features/ExponentialHeightFog.cpp, src/State.cpp
Lifecycle hooks for resource setup and cleanup; volumetric constant-buffer and GPU resource lifecycle; lazy compute-shader compilation with feature-dependent defines; prepass execution (grid/depth-distribution computation, buffer updates, compute dispatch, history management, temporal Halton jitter); UI controls and shadow-map capture integration into render state.
Shared Data Layout & Config Updates
package/Shaders/Common/SharedData.hlsli, features/Exponential Height Fog/Shaders/Features/ExponentialHeightFog.ini
ExponentialHeightFogSettings struct expanded with volumetric parameters (enable, grid sizing, distances, extinction, albedo/emissive, scattering intensities, history/jitter controls); INI version incremented to 1-3-0; directional anisotropy default changed from 0.7f0.2f.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
  • davo0411
  • alandtse

Poem

🐰 In froxel fields the soft mists play,

I hop through slices, light at bay,
With jittered Halton, history kept,
Shadows and sky in layers swept,
Exponential heights now glow—hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat(fog): volumetric fog' directly and clearly summarizes the main change: adding volumetric fog support to the Exponential Height Fog system, which aligns with the extensive shader additions, refactoring, and feature expansion documented in the raw summary.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

No actionable suggestions for changed features.

@jiayev jiayev added the deferred Defer to next cycle label May 17, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogCommon.hlsli (1)

1-3: Consider linking this feature PR to a tracking issue.

If there is an existing ticket, add a keyword like Implements #123 or `Addresses `#123 in the PR description for traceability.

As per coding guidelines **/*: “Issue References … Suggest adding appropriate GitHub keywords … for features.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/VolumetricFogCommon.hlsli around lines 1 - 3,
This PR adds the Exponential Height Fog volumetric feature (see header guard
__EXPONENTIAL_HEIGHT_FOG_VOLUMETRIC_COMMON_HLSLI__); update the PR description
to include a tracking-issue keyword such as "Implements #<issue-number>" or
"Addresses #<issue-number>" that references the related GitHub issue so the
feature is traceable per the coding guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli:
- Around line 168-172: The early return when fogDensity <= 0.0f in
ExponentialHeightFog.hlsli prematurely disables the whole feature; instead
remove the return and treat fogDensity==0 as disabling only
analytical/ground-based density while still allowing volumetric sampling and
blending to run. Modify the code around
SharedData::exponentialHeightFogSettings.fogDensity and fogHeightFalloff so that
if fogDensity <= 0.0f you set fogDensity to 0.0f (or branch to skip analytical
density computations) but do not return from the function, ensuring subsequent
volumetric sampling/blending code still executes.
- Around line 109-142: SampleVolumetricFog is not stereo-aware: it computes
volumeUV from screenPosition without stereo-conversion or eye-clamping so VR
right-eye reads the wrong froxels; update this path to mirror the world-space
sampler by threading an eyeIndex through
CombineVolumetricFog/GetExponentialHeightFog callers, convert screen-space UVs
with StereoToTextureUV (or equivalent) and apply the same eye clamping used near
the world-space sampler (use ClampToEye/eye-aware UV bounds) before calling
ExponentialHeightFogIntegratedLightScattering.SampleLevel; ensure the new
eyeIndex parameter is added to SampleVolumetricFog and propagated to its callers
so the correct texture slice is sampled per eye.

In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl:
- Around line 277-304: ComputeCellWorldPosition overwrites the out uint eyeIndex
when computing cellCornerWS, which then causes subsequent accesses like
light.positionWS[eyeIndex] to use the wrong eye for local-light sampling;
preserve the original eye index by saving it to a temporary (e.g., savedEye)
before calling ExponentialHeightFog::ComputeCellWorldPosition for the corner
sample, then use savedEye when indexing light.positionWS and any other per-eye
arrays during the localScattering accumulation.

In `@package/Shaders/Effect.hlsl`:
- Around line 891-894: The current EXP_HEIGHT_FOG path overwrites the vanilla
blend because blendedColor is first computed as lerp(lightColor,
vanillaFogColor, vanillaFogFactor.xxx) then immediately reset using lightColor
again; change the second assignment so it blends the existing blendedColor with
fogColor using fogFactor (i.e., lerp(blendedColor, fogColor, fogFactor.xxx)) so
the vanillaFogColor contribution is preserved; update the code around the
EXP_HEIGHT_FOG block (symbols: blendedColor, lightColor, vanillaFogColor,
vanillaFogFactor, fogColor, fogFactor, EXP_HEIGHT_FOG and the
ShouldDisableVanillaFog() path) accordingly.

In `@src/Features/ExponentialHeightFog.cpp`:
- Around line 256-295: The texture allocations (e.g., vBufferA,
conservativeDepth, conservativeDepthHistory, lightScattering,
lightScatteringHistory, integratedLightScattering created with
Texture3D/Texture2D and subsequent CreateSRV/CreateUAV calls) lack defensive
checks for allocation or SRV/UAV creation failures; wrap each allocation and
view-creation in a try/catch or check the underlying resource/creation-result
and handle failures by logging an error with context (e.g.,
"ExponentialHeightFog::ConservativeDepth"), releasing any partially-created
resources (resetting the related unique_ptr), and returning/propagating a clear
failure code/path instead of letting exceptions or invalid resources leak into
the rest of the system. Ensure checks reference the same symbols (vBufferA,
conservativeDepth, conservativeDepthHistory, lightScattering,
integratedLightScattering and their CreateSRV/CreateUAV calls) and provide
deterministic cleanup and error reporting.
- Around line 324-361: The shader getters (GetMaterialSetupCS,
GetConservativeDepthCS, GetLightScatteringCS, GetIntegrationCS) currently return
the raw result of Util::CompileShader which can be nullptr on failure; update
each getter to detect a nullptr from Util::CompileShader and log a warning once
(e.g., via processLogger or existing logging utility) including the shader path
and any defines used so failures are visible, and ensure the log only repeats on
the first failure for that specific shader to avoid spamming; keep the existing
lazy initialization and return behavior otherwise.

---

Nitpick comments:
In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/VolumetricFogCommon.hlsli:
- Around line 1-3: This PR adds the Exponential Height Fog volumetric feature
(see header guard __EXPONENTIAL_HEIGHT_FOG_VOLUMETRIC_COMMON_HLSLI__); update
the PR description to include a tracking-issue keyword such as "Implements
#<issue-number>" or "Addresses #<issue-number>" that references the related
GitHub issue so the feature is traceable per the coding guidelines.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 571aaa10-5f02-4da7-bb15-ca4a29bdd956

📥 Commits

Reviewing files that changed from the base of the PR and between 8c9b18c and 3b4ef75.

📒 Files selected for processing (16)
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogCSCommon.hlsli
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogCommon.hlsli
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogConservativeDepthCS.hlsl
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogIntegrationCS.hlsl
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogMaterialCS.hlsl
  • features/Exponential Height Fog/Shaders/Features/ExponentialHeightFog.ini
  • package/Shaders/Common/SharedData.hlsli
  • package/Shaders/Effect.hlsl
  • package/Shaders/ISSAOComposite.hlsl
  • package/Shaders/Lighting.hlsl
  • package/Shaders/Water.hlsl
  • src/Features/ExponentialHeightFog.cpp
  • src/Features/ExponentialHeightFog.h
  • src/State.cpp

Comment thread package/Shaders/Effect.hlsl
Comment thread src/Features/ExponentialHeightFog.cpp
Comment thread src/Features/ExponentialHeightFog.cpp
Comment thread src/Features/ExponentialHeightFog.cpp Outdated
@github-actions

github-actions Bot commented May 17, 2026

Copy link
Copy Markdown

✅ A pre-release build is available for this PR:
Download

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Features/ExponentialHeightFog.cpp`:
- Around line 680-733: The DrawSettings-exposed volumetric controls are missing
registry entries—add RegisterVariable calls for "volumetricFogStartDistance"
(WeatherVariables::FloatVariable), "volumetricFogNearFadeInDistance"
(WeatherVariables::FloatVariable), and
"volumetricDirectionalScatteringIntensity" (WeatherVariables::FloatVariable) in
the same block where other volumetric settings are registered (in
ExponentialHeightFog.cpp), binding each to the corresponding settings member
(settings.volumetricFogStartDistance, settings.volumetricFogNearFadeInDistance,
settings.volumetricDirectionalScatteringIntensity), and use sensible
defaults/ranges consistent with the existing volumetric variables (e.g.,
distances in the same scale/range as volumetricFogDistance and intensities
similar to volumetricSkyLightingIntensity), following the pattern of
std::make_shared<WeatherVariables::FloatVariable>(...) used for the other
entries.
- Around line 556-563: When temporal reprojection is disabled, avoid the
unnecessary GPU copies of the light scattering and conservative depth history;
update the block that currently calls
context->CopyResource(lightScatteringHistory->resource.get(),
lightScattering->resource.get()) and
context->CopyResource(conservativeDepthHistory->resource.get(),
conservativeDepth->resource.get()) so those CopyResource calls are skipped when
temporalReprojection is false, and ensure hasLightScatteringHistory and
hasConservativeDepthHistory are set correctly (false when reprojection is
disabled or when depthSrv is absent) to reflect that history SRVs will not be
consumed; modify the logic around temporalReprojection, lightScatteringHistory,
conservativeDepthHistory, depthSrv, hasLightScatteringHistory and
hasConservativeDepthHistory to gate the copies and flags accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc2181cc-0b25-4485-92f2-a645c020e633

📥 Commits

Reviewing files that changed from the base of the PR and between 3b4ef75 and f18d445.

📒 Files selected for processing (5)
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl
  • package/Shaders/Effect.hlsl
  • package/Shaders/ISSAOComposite.hlsl
  • src/Features/ExponentialHeightFog.cpp
🚧 Files skipped from review as they are similar to previous changes (4)
  • package/Shaders/ISSAOComposite.hlsl
  • package/Shaders/Effect.hlsl
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli

Comment thread src/Features/ExponentialHeightFog.cpp Outdated
Comment thread src/Features/ExponentialHeightFog.cpp

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/State.cpp (1)

100-101: ⚠️ Potential issue | 🟠 Major

Gate ExponentialHeightFog::CaptureDirectionalShadowMap() to active volumetric fog
src/Features/ExponentialHeightFog.cpp’s ExponentialHeightFog::CaptureDirectionalShadowMap() unconditionally reads PS shader resource slot 4 and copies it into directionalShadowMap; it does not check settings.volumetricFogEnabled/settings.enabled and has no lastPrepassFrame/frameCount guard. As a result, the call from src/State.cpp’s shadowmask hot path will still do work when volumetric fog is off, risking avoidable per-frame overhead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/State.cpp` around lines 100 - 101, The call to
ExponentialHeightFog::CaptureDirectionalShadowMap() is currently unconditional
and causes work even when volumetric fog is disabled; modify the logic so
CaptureDirectionalShadowMap() early-returns when fog is not active (check
settings.enabled and settings.volumetricFogEnabled) and add a frame guard using
lastPrepassFrame vs current frameCount to avoid duplicate work per-frame;
alternatively (or in addition) gate the call site in State.cpp (where
globals::features::exponentialHeightFog.CaptureDirectionalShadowMap() is
invoked) so it only calls when exponentialHeightFog.loaded && settings.enabled
&& settings.volumetricFogEnabled and the lastPrepassFrame/frameCount guard
prevents repeated captures. Ensure directionalShadowMap reads only occur behind
these guards.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/State.cpp`:
- Around line 100-101: The call to
ExponentialHeightFog::CaptureDirectionalShadowMap() is currently unconditional
and causes work even when volumetric fog is disabled; modify the logic so
CaptureDirectionalShadowMap() early-returns when fog is not active (check
settings.enabled and settings.volumetricFogEnabled) and add a frame guard using
lastPrepassFrame vs current frameCount to avoid duplicate work per-frame;
alternatively (or in addition) gate the call site in State.cpp (where
globals::features::exponentialHeightFog.CaptureDirectionalShadowMap() is
invoked) so it only calls when exponentialHeightFog.loaded && settings.enabled
&& settings.volumetricFogEnabled and the lastPrepassFrame/frameCount guard
prevents repeated captures. Ensure directionalShadowMap reads only occur behind
these guards.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ae30107-a690-4304-9a27-d1eea5b58073

📥 Commits

Reviewing files that changed from the base of the PR and between f18d445 and e58fb23.

📒 Files selected for processing (3)
  • package/Shaders/Common/SharedData.hlsli
  • src/Features/ExponentialHeightFog.h
  • src/State.cpp

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl (1)

337-350: Directional HG phase correction already rescaled at composite

  • ExponentialHeightFog.hlsli rescales the baked isotropic factor at composite: ApplyDirectionalPhaseCorrection computes perPixelPhase = HenyeyGreenstein(cosTheta, g) and applies correction = lerp(1.0, perPixelPhase / (1/(4*PI)), dirFraction) (i.e., it divides out isotropicPhase before applying HG), and CombineVolumetricFog() runs this after SampleVolumetricFog().
  • Minor: the CS comment claims the HG phase is applied during compositing in SampleVolumetricFog(), but the phase correction is actually applied in CombineVolumetricFog() via ApplyDirectionalPhaseCorrection().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl around
lines 337 - 350, The comment in VolumetricFogLightScatteringCS.hlsl incorrectly
states that the per-pixel HG phase is applied during compositing in
SampleVolumetricFog() and implies no rescaling; update the comment to state that
the baked isotropic factor (directionalPhase) is rescaled later in
CombineVolumetricFog() via ApplyDirectionalPhaseCorrection (defined in
ExponentialHeightFog.hlsli), and ensure you do not double-rescale the phase
(i.e., keep directionalPhase = 1/(4*PI) here as the baked isotropic factor and
document that ApplyDirectionalPhaseCorrection divides this out and applies the
per-pixel HenyeyGreenstein correction).
features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli (2)

175-197: ⚡ Quick win

viewDirection wiring is correct; consider de-duplicating the two combiners.

The added viewDirection parameter and ApplyDirectionalPhaseCorrection call are correct and self-contained (only caller is Line 279). The two CombineVolumetricFog overloads are now identical except for the sampler call — the blend body is copy-pasted, so any future fix must be applied twice. Extracting the blend keeps them in sync.

♻️ Extract shared blend helper
+	float4 BlendVolumetricAndAnalyticalFog(float4 volumetricFog, float4 analyticalFog)
+	{
+		float analyticalTransmittance = 1.0f - analyticalFog.w;
+		float combinedTransmittance = volumetricFog.a * analyticalTransmittance;
+		float combinedOpacity = saturate(1.0f - combinedTransmittance);
+		float3 analyticalPremultiplied = analyticalFog.rgb * analyticalFog.w;
+		float3 combinedPremultiplied = volumetricFog.rgb + volumetricFog.a * analyticalPremultiplied;
+		return float4(combinedOpacity > 1e-4f ? combinedPremultiplied / combinedOpacity : float3(0.0f, 0.0f, 0.0f), combinedOpacity);
+	}
+
 	float4 CombineVolumetricFog(float4 analyticalFog, float3 positionWS, uint eyeIndex, float3 viewDirection)
 	{
 		float4 volumetricFog = SampleVolumetricFog(positionWS, eyeIndex);
 		volumetricFog = ApplyDirectionalPhaseCorrection(volumetricFog, viewDirection);
-		float analyticalTransmittance = 1.0f - analyticalFog.w;
-		float combinedTransmittance = volumetricFog.a * analyticalTransmittance;
-		float combinedOpacity = saturate(1.0f - combinedTransmittance);
-		float3 analyticalPremultiplied = analyticalFog.rgb * analyticalFog.w;
-		float3 combinedPremultiplied = volumetricFog.rgb + volumetricFog.a * analyticalPremultiplied;
-		return float4(combinedOpacity > 1e-4f ? combinedPremultiplied / combinedOpacity : float3(0.0f, 0.0f, 0.0f), combinedOpacity);
+		return BlendVolumetricAndAnalyticalFog(volumetricFog, analyticalFog);
 	}

 	float4 CombineVolumetricFog(float4 analyticalFog, float4 screenPosition, uint eyeIndex, float3 viewDirection)
 	{
 		float4 volumetricFog = SampleVolumetricFog(screenPosition, eyeIndex);
 		volumetricFog = ApplyDirectionalPhaseCorrection(volumetricFog, viewDirection);
-		float analyticalTransmittance = 1.0f - analyticalFog.w;
-		float combinedTransmittance = volumetricFog.a * analyticalTransmittance;
-		float combinedOpacity = saturate(1.0f - combinedTransmittance);
-		float3 analyticalPremultiplied = analyticalFog.rgb * analyticalFog.w;
-		float3 combinedPremultiplied = volumetricFog.rgb + volumetricFog.a * analyticalPremultiplied;
-		return float4(combinedOpacity > 1e-4f ? combinedPremultiplied / combinedOpacity : float3(0.0f, 0.0f, 0.0f), combinedOpacity);
+		return BlendVolumetricAndAnalyticalFog(volumetricFog, analyticalFog);
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli around lines 175 -
197, The two CombineVolumetricFog overloads duplicate identical blend logic
after calling SampleVolumetricFog and ApplyDirectionalPhaseCorrection; extract
that shared blending code into a new helper (e.g., BlendVolumetricAndAnalytical
or CombineFogBlends) that takes the computed volumetricFog and analyticalFog and
returns the final float4, then call that helper from both CombineVolumetricFog
overloads (keeping the existing SampleVolumetricFog(screenPosition/positionWS)
and ApplyDirectionalPhaseCorrection calls intact); reference
SampleVolumetricFog, ApplyDirectionalPhaseCorrection, and the two
CombineVolumetricFog functions when implementing the refactor so future fixes
apply in one place.

149-173: Directional phase correction depends on the compute-side contract—this matches
VolumetricFogLightScatteringCS stores the directional term with isotropic directionalPhase = 1/(4π) (HG is applied later during compositing in SampleVolumetricFog()), and it scales directional vs sky contributions using volumetricDirectionalScatteringIntensity and volumetricSkyLightingIntensity respectively—so this correction shouldn’t double-apply phase.
Accuracy may still be approximate because ApplyDirectionalPhaseCorrection’s dirFraction uses only those intensity scalars (and DirLightColor luminance), while the compute’s sky contribution comes from view-/visibility-dependent GetIBLColorOccluded(..., skyVisibility) (plus other terms like local/emissive).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli around lines 149 -
173, The current ApplyDirectionalPhaseCorrection uses only scalar intensity
settings and DirLightColor luminance to compute dirFraction, which can
mis-estimate directional vs sky contributions because the compute shader's sky
term is view- and visibility-dependent; update ApplyDirectionalPhaseCorrection
to compute skyStrength from the actual occluded sky contribution (e.g., call or
accept the result of GetIBLColorOccluded(..., skyVisibility) and use its
luminance) instead of relying solely on
exponentialHeightFogSettings.volumetricSkyLightingIntensity, then recompute
dirFraction = saturate(dirStrength / max(dirStrength + skyLuminance, 1e-5f)) and
apply the same correction logic; reference ApplyDirectionalPhaseCorrection,
GetIBLColorOccluded, VolumetricFogLightScatteringCS, SampleVolumetricFog,
SharedData::exponentialHeightFogSettings.volumetricSkyLightingIntensity and
volumetricDirectionalScatteringIntensity when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli:
- Around line 175-197: The two CombineVolumetricFog overloads duplicate
identical blend logic after calling SampleVolumetricFog and
ApplyDirectionalPhaseCorrection; extract that shared blending code into a new
helper (e.g., BlendVolumetricAndAnalytical or CombineFogBlends) that takes the
computed volumetricFog and analyticalFog and returns the final float4, then call
that helper from both CombineVolumetricFog overloads (keeping the existing
SampleVolumetricFog(screenPosition/positionWS) and
ApplyDirectionalPhaseCorrection calls intact); reference SampleVolumetricFog,
ApplyDirectionalPhaseCorrection, and the two CombineVolumetricFog functions when
implementing the refactor so future fixes apply in one place.
- Around line 149-173: The current ApplyDirectionalPhaseCorrection uses only
scalar intensity settings and DirLightColor luminance to compute dirFraction,
which can mis-estimate directional vs sky contributions because the compute
shader's sky term is view- and visibility-dependent; update
ApplyDirectionalPhaseCorrection to compute skyStrength from the actual occluded
sky contribution (e.g., call or accept the result of GetIBLColorOccluded(...,
skyVisibility) and use its luminance) instead of relying solely on
exponentialHeightFogSettings.volumetricSkyLightingIntensity, then recompute
dirFraction = saturate(dirStrength / max(dirStrength + skyLuminance, 1e-5f)) and
apply the same correction logic; reference ApplyDirectionalPhaseCorrection,
GetIBLColorOccluded, VolumetricFogLightScatteringCS, SampleVolumetricFog,
SharedData::exponentialHeightFogSettings.volumetricSkyLightingIntensity and
volumetricDirectionalScatteringIntensity when making the change.

In `@features/Exponential` Height
Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl:
- Around line 337-350: The comment in VolumetricFogLightScatteringCS.hlsl
incorrectly states that the per-pixel HG phase is applied during compositing in
SampleVolumetricFog() and implies no rescaling; update the comment to state that
the baked isotropic factor (directionalPhase) is rescaled later in
CombineVolumetricFog() via ApplyDirectionalPhaseCorrection (defined in
ExponentialHeightFog.hlsli), and ensure you do not double-rescale the phase
(i.e., keep directionalPhase = 1/(4*PI) here as the baked isotropic factor and
document that ApplyDirectionalPhaseCorrection divides this out and applies the
per-pixel HenyeyGreenstein correction).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bbaa6732-0837-4d9c-aa7c-3f0a617ccafd

📥 Commits

Reviewing files that changed from the base of the PR and between d893c25 and a30e14e.

📒 Files selected for processing (2)
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/ExponentialHeightFog.hlsli
  • features/Exponential Height Fog/Shaders/ExponentialHeightFog/VolumetricFogLightScatteringCS.hlsl

@SkrubbySkrubInAShrub SkrubbySkrubInAShrub removed the deferred Defer to next cycle label May 31, 2026
@SkrubbySkrubInAShrub

Copy link
Copy Markdown
Collaborator

please resolve the AI comments

@davo0411 davo0411 merged commit be22b78 into community-shaders:dev Jun 3, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants